Skip to content

Integration dev - #10

Merged
adamfreedman merged 8 commits into
mainfrom
integration_dev
Jul 16, 2026
Merged

Integration dev#10
adamfreedman merged 8 commits into
mainfrom
integration_dev

Conversation

@adamfreedman

Copy link
Copy Markdown
Contributor

integrated downsampling workflow

adamfreedman and others added 8 commits June 30, 2026 15:49
…icate

bootstrap_clusters ran all downsampling replicates sequentially in one
job, and its fixed runtime (not scaled by attempt) was timing out on
large Seurat objects that used to work fine before other refactoring.

Split the rule into downsample_cluster_replicate (one SLURM job per
downsample_target/replicate pair) and downsample_clusters (a cheap
gather step that concatenates the per-replicate TSVs into the final
summary table). Verified the new pipeline reproduces the existing
reference outputs byte-for-byte and the downsampling test suite passes.
240 minutes was an unjustified guess. A single replicate only
downsamples barcodes and reruns clustering, which takes a few minutes
even on a laptop; the old monolithic job's own numbers (100 replicates
serially exceeding 960 min) implied well under 15 min/replicate.
Requesting far more runtime than needed also hurts SLURM queue time.
… job per target

do.call(SCTransform, sct_args) embedded the full Seurat object as a literal
value in the call frame; Seurat's internal Parenting() helper stringifies the
entire call stack on each progress tick during ScaleData's regression step,
which made that stringification catastrophically slow (40+ min hangs, up to
256GB memory requested on retry) purely as an artifact of using do.call
instead of a direct call. Calling SCTransform directly drops real per-
replicate runtime to ~2 minutes and ~10-15GB.

With replicates that cheap, running each as its own SLURM job meant queue
wait time (median ~80min) vastly exceeded actual compute time (~2min) per
job. Collapsed the two-rule replicate/aggregate split back into a single
downsample_clusters rule per target that loops over all replicates in one
process and writes the combined summary table directly, cutting job count
for this stage from ~2300 to ~23. Also throttled SLURM job submission/status-
check rate in the runner script to avoid overloading the scheduler under
heavy job counts.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
temp() on find_markers' output wasn't reliably triggering cleanup across
this workflow's repeated crash/rerun history: several prefixes had already
completed combine_markers successfully yet still had their per-cluster
_markergenes_cluster{N}.csv files left on disk. Delete them explicitly via
input.markers in combine_markers' shell block instead of relying solely on
temp() deletion.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ion tests in CI automatically

The existing downsampling tests only ran the rule against a single tiny
fixture (300 cells) and asserted correctness, never runtime, so the
do.call(SCTransform, ...) performance regression (see the recent
downsample_clusters.R/downsample_clusters.smk fix) was invisible to them
regardless of whether they ran: a data-size-dependent slowdown has nothing
to bite on at a single, tiny scale, and produces byte-identical output.

Add a synthetic mid-scale fixture (tests/downsampling/make_medium_fixture.R,
2500 cells via stratified resampling of the existing fixture) and a new
test that runs downsample_clusters on both fixture sizes and asserts the
wall-clock ratio stays within a generous multiple of the ~8.3x cell-count
ratio - loose enough for normal scaling, tight enough to have caught this
exact bug.

Also discovered that downsample-rule/downsample-workflow (and now
downsample-scaling) were gated behind workflow_dispatch and never ran on
regular pushes/PRs, which was a second, independent reason this could slip
through. All three now run as unconditional jobs.

Fixed test_downsampling_snakemake_dry_run.py, which was still asserting on
the now-removed downsample_cluster_replicate rule name from the earlier
rule-merge and had been silently broken since.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t_config_validation.py

These two assertions were missed when the downsample_cluster_replicate rule
was merged into downsample_clusters; unlike the equivalent fix already made
in tests/downsampling/test_downsampling_snakemake_dry_run.py, this file runs
in the default (non-opt-in) pytest tests -q CI job, so it broke that job on
every push since the rule-merge commit (GitHub Actions runs #37, #38, #39).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
actions/checkout@v4, actions/cache@v4, and mamba-org/setup-micromamba@v2
all still declare node20, which GitHub now runs on a forced/deprecated
Node 24 runtime with warnings. Bump to actions/checkout@v7, actions/cache@v6,
and mamba-org/setup-micromamba@v3, all of which target node24 natively.
No breaking changes apply to this workflow's usage (basic checkout, plain
path/key/restore-keys caching, environment-file-based micromamba setup).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@adamfreedman
adamfreedman merged commit dbd07e3 into main Jul 16, 2026
13 checks passed
@adamfreedman
adamfreedman deleted the integration_dev branch July 16, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant